Skip to content

An OpenFeign Client for Dapr Invokes #1294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

lony2003
Copy link

@lony2003 lony2003 commented Apr 2, 2025

Description

A OpenFeign Client and it's AutoConfiguration using Dapr Invoke APIs.

Splited into two librarys, dapr-openfeign-client is the pure client of OpenFeign, and dapr-spring-openfeign is the plugin of Spring Cloud OpenFeign.

Currently only HTTP Invoke Method and Invoke Binding supported, and Invoke Binding is not always recommend to use this client, depends on what binding component is.

Issue reference

this PR will close: #1181

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

lony2003 added 6 commits April 1, 2025 21:58
A OpenFeign Client and it's AutoConfiguration using Dapr Invoke APIs.

Splited into two librarys, dapr-openfeign-client is the pure client of OpenFeign, and dapr-spring-openfeign is the plugin of Spring Cloud OpenFeign.

Currently only HTTP Invoke Method and Invoke Binding supported, and Invoke Binding is not always recommend to use this client, depends on what binding component is.

Signed-off-by: lony2003 <[email protected]>
A OpenFeign Client and it's AutoConfiguration using Dapr Invoke APIs.

Splited into two librarys, dapr-openfeign-client is the pure client of OpenFeign, and dapr-spring-openfeign is the plugin of Spring Cloud OpenFeign.

Currently only HTTP Invoke Method and Invoke Binding supported, and Invoke Binding is not always recommend to use this client, depends on what binding component is.

Signed-off-by: lony2003 <[email protected]>
A OpenFeign Client and it's AutoConfiguration using Dapr Invoke APIs.

Splited into two librarys, dapr-openfeign-client is the pure client of OpenFeign, and dapr-spring-openfeign is the plugin of Spring Cloud OpenFeign.

Currently only HTTP Invoke Method and Invoke Binding supported, and Invoke Binding is not always recommend to use this client, depends on what binding component is.

Signed-off-by: lony2003 <[email protected]>
this example can redirect order request to producer-app. It needs producer-app to run properly.

Signed-off-by: lony2003 <[email protected]>
this test tests the PostgreSQL binding of dapr.

Signed-off-by: lony2003 <[email protected]>
@lony2003 lony2003 requested review from a team as code owners April 2, 2025 12:53
@salaboy
Copy link
Collaborator

salaboy commented Apr 2, 2025

I haven't reviewed this yet @lony2003 , but I am more convinced that we need this .. @artur-ciocanu @mcruzdev can you help me to check this PR ?

Fix the example doc to make openfeign app works in mm.py

Signed-off-by: lony2003 <[email protected]>
@salaboy
Copy link
Collaborator

salaboy commented Apr 7, 2025

@lony2003 I've just reviewed this PR and it is looking awesome..
I am currently working on testing multiple sidecars using Testcontainers (for service to service invocation) scenarios, so I will probably tag you in those examples when I have them running. I think this PR is ok (as soon as there are no bean conflicts), then we can create some examples in spring-boot-examples showing the feign clients when we can test service to service invocations with multiple sidecars.

…riginal Targeter beans instead of override them.

Signed-off-by: lony2003 <[email protected]>
@lony2003
Copy link
Author

lony2003 commented Apr 7, 2025

@salaboy I have changed the Targeter part to using BeanPostProcesser to handle them, so now no confliects will happen.

@lony2003 lony2003 requested a review from cicoyle April 8, 2025 05:53
@lony2003
Copy link
Author

lony2003 commented Apr 8, 2025

@cicoyle I have checked all files and add headers for them, and I have solved the problem that makes IT failed. Please re-trigger the CI.

@cicoyle
Copy link
Contributor

cicoyle commented Apr 22, 2025

Hey @lony2003 - thanks, before I can retrigger CI it says there are conflicts needing to be resolved. Mind resolving them locally and pushing? Then we'll be able to proceed 🙏🏻

@lony2003 lony2003 force-pushed the development-openfeign branch from 8580167 to a330ef2 Compare April 23, 2025 01:29
@lony2003 lony2003 force-pushed the development-openfeign branch 5 times, most recently from 6d1e70a to 1767276 Compare April 23, 2025 04:40
@lony2003 lony2003 force-pushed the development-openfeign branch 2 times, most recently from 3eaa83e to c381e44 Compare April 23, 2025 05:10
… client bean autowired

Job SDK ITs have a daprPreviewClient bean defined, as SpringBoot bean search will first get beans that impl the interface and get all beans that matches the bean's class, when openfeign autowire DaprClient, there is a conflict between daprClient and daprPreviewClient.

Change the AutoConfiguration behavior to make a more strict rule for making AutoConfiguration run

Signed-off-by: lony2003 <[email protected]>
@lony2003 lony2003 force-pushed the development-openfeign branch 4 times, most recently from 00fae6b to bc6d98c Compare April 23, 2025 06:10
… client bean autowired

Job SDK ITs have a daprPreviewClient bean defined, as SpringBoot bean search will first get beans that impl the interface and get all beans that matches the bean's class, when openfeign autowire DaprClient, there is a conflict between daprClient and daprPreviewClient.

Change the AutoConfiguration behavior to make a more strict rule for making AutoConfiguration run

Signed-off-by: lony2003 <[email protected]>
@lony2003
Copy link
Author

@cicoyle conflict solved.

Copy link

codecov bot commented May 6, 2025

Codecov Report

Attention: Patch coverage is 65.58140% with 74 lines in your changes missing coverage. Please review.

Project coverage is 76.75%. Comparing base (d759c53) to head (e8a35cc).
Report is 138 commits behind head on master.

Files with missing lines Patch % Lines
...main/java/io/dapr/feign/DaprInvokeFeignClient.java 74.49% 28 Missing and 10 partials ⚠️
.../spring/openfeign/targeter/DaprClientTargeter.java 0.00% 20 Missing ⚠️
.../targeter/DaprClientTargeterBeanPostProcessor.java 0.00% 6 Missing ⚠️
...feign/autoconfigure/DaprFeignClientProperties.java 55.55% 4 Missing ⚠️
...onfigure/FeignClientAnnoationEnabledCondition.java 50.00% 2 Missing and 1 partial ⚠️
...gboot/examples/openfeign/OpenFeignApplication.java 33.33% 2 Missing ⚠️
...a/io/dapr/springboot/examples/openfeign/Order.java 91.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1294      +/-   ##
============================================
- Coverage     76.91%   76.75%   -0.16%     
- Complexity     1592     1769     +177     
============================================
  Files           145      209      +64     
  Lines          4843     5498     +655     
  Branches        562      597      +35     
============================================
+ Hits           3725     4220     +495     
- Misses          821      957     +136     
- Partials        297      321      +24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@salaboy
Copy link
Collaborator

salaboy commented May 7, 2025

@lony2003 I am happy to see the CI happy for this PR, can you check the codecov?

Copy link
Collaborator

@salaboy salaboy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use With Feign
3 participants